Skip to content

Add the monitoring/kpi_gate skill bundle implementing the issue #317 interface - #318

Open
mrmasa88 wants to merge 3 commits into
ARPAHLS:mainfrom
0x-AO-Protocol:feat/issue-317-kpi-gate
Open

Add the monitoring/kpi_gate skill bundle implementing the issue #317 interface#318
mrmasa88 wants to merge 3 commits into
ARPAHLS:mainfrom
0x-AO-Protocol:feat/issue-317-kpi-gate

Conversation

@mrmasa88

@mrmasa88 mrmasa88 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Description

Adds the monitoring/kpi_gate registry skill implementing the interface agreed in #317 (three input JSON Schemas, output contract, four-stage fail-closed validation order, agent-loop contract, and the end-to-end example). Design rationale and the frozen interface live in the #317 comments and are not re-argued here.

Acceptance criteria mapped to the diff:

  • Three-state vocabulary (error / warning / insufficient_data with reason codes) — skills/monitoring/kpi_gate/skill.py; branch coverage in test_skill.py (28 offline tests)
  • Strict schemas, closed rule set, fail-closed errors — stdlib validation mirroring the three [New Skill]: monitoring/funnel_monitor — deterministic funnel health evaluation against charter thresholds #317 schemas (shipped verbatim under schemas/ as reference documents; requirements: [] stays empty by design, so no runtime jsonschema dependency); closed error registry INVALID_METRICS_SCHEMA, INVALID_POLICY_SCHEMA, INVALID_BENCHMARKS_SCHEMA, NO_METRICS_PROVIDED, UNKNOWN_METRIC_KEY, UNKNOWN_RULE_METRIC, UNKNOWN_DENOMINATOR_METRIC, BENCHMARK_VERSION_MISSING, BENCHMARK_REF_UNRESOLVED — one dedicated test per code, including the dependentRequired rejection
  • End-to-end example — the [New Skill]: monitoring/funnel_monitor — deterministic funnel health evaluation against charter thresholds #317-posted snapshot/charter/benchmarks fixtures ship in the bundle; test_e2e_matches_expected_findings_exactly asserts exact equality with the posted expected findings
  • Determinism — test_repeat_execution_is_bit_identical (serialized-output equality); no network modules imported (statically asserted)
  • Honesty floor — insufficient_data iff the declared floor is unmet, boundary-tested at the minimum denominator (25 computes, 24 refuses); declared metrics only, never inferred
  • Agent-loop contract — stated in instructions.md and on the catalog page (error → host blocks until operator override; warning → surface, never block; insufficient_data → treat as absent, never substitute)
  • kb/ benchmarks-as-data — kb/benchmarks_demo.json (timestamped, sourced, versioned; synthetic demo values); revisions land as data-only PRs, proposer-maintained
  • Complete documentation surface — catalog page with version metadata, Skill history, and Usage Examples for all five providers; registry index row; agent-loops matrix row; extras sync

No runnable script is added under examples/, so examples/README.md is intentionally untouched (the in-bundle fixtures are the end-to-end example pack per #317).

External impact (quantified)

  • New (27 files): skills/monitoring/kpi_gate/ — 25 bundle files (__init__.py, manifest.yaml, skill.py, instructions.md, card.json, test_skill.py, 3 reference schemas, 1 kb demo data file, 15 fixtures) — plus docs/skills/kpi_gate.md and tests/fixtures/card_ui_schema/monitoring__kpi_gate.json
  • Existing files changed (4, one hunk each): CHANGELOG.md 1 entry under [Unreleased]; docs/skills/README.md 1 index row; docs/usage/agent_loops.md 1 matrix row; pyproject.toml 1 generated extras line (monitoring_kpi_gate = [], from scripts/sync_extras.py)
  • skillware/core/ and all other skills: untouched

Removal procedure

Deleting skills/monitoring/kpi_gate/, docs/skills/kpi_gate.md, and tests/fixtures/card_ui_schema/monitoring__kpi_gate.json, then reverting the four one-hunk edits listed above (CHANGELOG entry, README row, agent-loops row, generated extras line), removes this contribution completely; no other file depends on it.

Notes for review

Type of Change

  • New Skill — new registry bundle under skills/
  • Skill Upgrade — changes to an existing skill under skills/
  • Bug Fix — incorrect runtime or framework behavior
  • Documentation — docs, README, CONTRIBUTING only
  • Framework Feature — skillware/core/ loader, env, adapters
  • CLI — skillware/cli.py, docs/usage/cli.md
  • Examples — examples/*.py, agent loops, examples/README.md
  • Packaging — PyPI wheel, pyproject.toml, MANIFEST.in
  • RFC / meta — templates, labels, CI, or large design doc

Checklist (all PRs)

  • Linked GitHub issue (Fixes #… or Refs #…)
  • Scope matches the issue — no unrelated refactors
  • python -m black --check . and flake8 pass locally (or CI-equivalent subset)
  • pytest skills/ and pytest tests/ pass locally when relevant (286 and 259 passed)
  • CHANGELOG.md updated under [Unreleased] when user-visible behavior changes
  • examples/README.md updated if this PR adds, renames, or removes a runnable script (no script added; file untouched)
  • Ran pytest tests/test_registry_docs.py when skills, examples index, or agent-loops matrix changed (9 passed)

New or updated skill

Bundle and metadata

  • Skill at skills/<category>/<skill_name>/ (from templates/python_skill/ or equivalent)
  • manifest.yaml: name (full ID), version, description, parameters, constitution, real issuer
  • Optional: short_description, issuer.github, issuer.org, requirements, env_vars (short_description, github, org: AO, requirements: []; no env_vars — fully offline)

Logic, cognition, tests

  • Deterministic skill.py (no ad-hoc LLM-generated execution paths)
  • instructions.md explains when and how to use the skill
  • card.json issuer matches manifest when present
  • test_skill.py covers execution and schema expectations
  • SkillLoader.load_skill("<category>/<skill_name>") succeeds (or deps documented)

Documentation and catalog

  • docs/skills/<skill_name>.md and row in docs/skills/README.md
  • Usage Examples for Gemini, Claude, OpenAI, DeepSeek, Ollama per skill usage template

Constitution and safety (skills only)

Evaluate-only: no data fetching, no side effects, no automated remediation, no network in execute(). Refuses rather than guesses (insufficient_data with reason codes; no default substitution). Every finding carries code, metric, threshold, and observed value. Honest limits (no data acquisition, causal inference, or threshold optimization) documented in instructions.md and the catalog page.

Related Issues

Fixes #317

@mrmasa88
Add the monitoring/kpi_gate skill bundle implementing the issue ARPAH…
1c1532b 
…LS#317 interface.

The bundle evaluates a metrics snapshot against an operator-maintained policy charter and optional versioned benchmark data, returning error, warning, and insufficient_data findings with fail-closed contract errors from a closed registry. Validation follows the four-stage order frozen in issue ARPAHLS#317, uses stdlib checks only (requirements stay empty), and ships the three reference JSON Schemas, demo benchmark data, end-to-end and fail-closed fixtures, 28 offline bundle tests, and the card UI schema fixture. Refs ARPAHLS#317
@mrmasa88
Document the monitoring/kpi_gate skill across the catalog and registr…
93ddb57 
…y indexes.

Adds the catalog page with version metadata, the closed error registry, usage examples for all five providers, and an initial skill history row; adds the index row in docs/skills/README.md, the reference-scripts matrix row in docs/usage/agent_loops.md, the CHANGELOG entry under Unreleased, and the generated monitoring_kpi_gate extras line from scripts/sync_extras.py. Refs ARPAHLS#317
@rosspeili

rosspeili commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Thanks @mrmasa88, this is huge, detailed, yet solid work on the discussion on #317.

Before merge (please):

  1. Drop any Co-authored-by: AI trailers on commits (repo policy, human attribution only). Your last commit is with Claude.
  2. instructions.md tone, opening line is close to persona-style (You are using…). Please align with skill-context guidance: lead with registry ID + contract/limits (catalog page style), like other monitoring skills.
  3. Issue [New Skill]: monitoring/funnel_monitor — deterministic funnel health evaluation against charter thresholds #317 title/Skill ID, PR ships monitoring/kpi_gate (not funnel_monitor). Fine if that rename was agreed in the thread; we’ll update the issue field on merge, confirm that’s intentional.
  4. No examples/*_demo.py, I guess acceptable given in-bundle fixtures, optional later local-execute demo + smoke row.
  • __init__.py export of KpiGateSkill optional but nice for consistency. I would include both in this PR.

Also run black and flake8, it showed 4 files if not more would be affected on my machine.

Once CI is green LGTM to merge. Nice work as always <3

@mrmasa88

mrmasa88 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks Ross! @rosspeili

On (3): yes, intentional — the rename came out of the Path A
split we agreed in #317 (generic evaluator, coaching funnel demoted to
example pack), and you confirmed kpi_gate. Happy for you to
update the issue field on merge.

Working through the rest now — trailers, instructions opener, examples demo

  • smoke row, __init__ export, and a clean black/flake8 pass.

@mrmasa88
Address the review feedback on the monitoring/kpi_gate bundle.
6cde2a2 
Rewrites the instructions.md opening to the skill-context style used by the
other monitoring skills (registry ID, deterministic contract, limits) instead
of a persona-style opener; exports KpiGateSkill from the package __init__;
adds the offline local-execute demo examples/kpi_gate_demo.py with its CI
smoke row and examples index entry. Repo-wide black and flake8 pass with no
changes outside the bundle. Refs ARPAHLS#317
@mrmasa88

mrmasa88 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Hey! @rosspeili

Thanks Ross — all six addressed, pushed as a follow-up commit (no history rewrite; see (1) below).

  1. Co-authored-by trailers — I could not reproduce this. Both commits on the branch are clean: git log --format='%B' | grep -i co-authored returns nothing, and the raw commit objects carry no trailer. Could you re-check on the current head? Possible it was a stale view. Setting is now hard-disabled on my side regardless (includeCoAuthoredBy: false), so nothing further can slip in. Happy to squash if you'd still prefer a clean single commit — say the word and it's one command.

  2. instructions.md opener — rewritten against the monitoring/token_limiter structure: registry ID first, then the deterministic contract, then limits. No persona framing.

  3. Skill ID rename — yes, intentional. It came out of the Path A split we agreed in [New Skill]: monitoring/funnel_monitor — deterministic funnel health evaluation against charter thresholds #317 (generic evaluator; the coaching funnel demoted to an example pack), and you confirmed kpi_gate separately. Please do update the issue field on merge.

  4. examples/kpi_gate_demo.py — added, fully offline against the in-bundle fixtures, three scenarios: the E2E case (error + warning + insufficient_data in one run), fail-closed on empty metrics, and fail-closed on an invalid charter. Plus the smoke row in tests/test_examples_smoke.py and the examples/README.md index line.

  5. __init__.py — KpiGateSkill now exported with __all__.

  6. black / flake8 — run from the repo root with no arguments: 174 files unchanged, 0 findings, zero diff outside the bundle. I could not reproduce the four-file result — possibly a black version difference between our environments? Happy to match whatever version CI pins if that's the cleaner fix.

CI is green on the new head (6cde2a2): all six checks pass — build 3.10 / 3.11 / 3.12, wheel-smoke, CodeQL, Analyze (python).

Locally, gates were re-run under an isolated SKILLWARE_CONFIG_DIR: bundle 28 passed, issuer 5, identity 2, registry-docs 9, the new smoke row 1, sync_extras --check clean, wheel smoke exit 0 with kpi_gate among the loaded skills. Two local failures in examples-smoke are pre-existing (mental_coach, bg_remover optional deps missing on my machine) and unrelated to this change — CI covers them fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[New Skill]: monitoring/funnel_monitor — deterministic funnel health evaluation against charter thresholds

2 participants

@mrmasa88 @rosspeili